fix(coordinator): reconcile durable runtime events - #4702
Conversation
b18f3b9 to
9e483b1
Compare
|
Refreshing exact-head contract after rebasing and completing the required PR body. |
Yeachan-Heo
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED for exact head 9e483b1 over base 7e256d6.
gajae.pr-review-verdict.v1 needs-human sha256:fbfb50a663649008a3985502b180c47a22f6844c7f4649c7d33c8935c6e015fd reviewer:critic reviewer-id:Yeachan-Heo evidence:exact-head-source-review-and-focused-validation
Blocking implementation findings:
- Accepted workflow.gate_answer updates canonical state but emits no question.answered outbox/journal event (server.ts:6968-6991), so watch_events/Codex wake consumers cannot observe answer completion.
- Q12 reconciliation always inspects only the first two roster entries and ignores the persisted scheduler cursor (server.ts:5218-5246), which can starve later sessions.
- Event-triggered reconciliation truncates to four sessions without durable continuation while watch cursors advance (server.ts:5190-5216), allowing unreconciled sessions to be skipped.
- Retained-delivery round-robin scans can fail to persist progress on empty bounded scans (question-state.ts:371-420; server.ts:2690-2718).
- watch_events has timeout/teardown races: watcher setup can continue after settlement and the initial journal read does not receive the abort signal (server.ts:2559-2590, 5659-5684).
- Answer terminal_uncertain recovery is sealed by the outer idempotency response, preventing same-key recovery (server.ts:6579-6580, 6628-6649, 7038-7045).
- Detached Codex wake work is not owned/awaited by server.close, and compactTransaction has no callsite (server.ts:2658-2669, 7239-7242; question-state.ts:1148-1175).
Exact-head evidence:
- Event-focused coordinator tests: 15 passed, 0 failed (bun test packages/coding-agent/test/coordinator-mcp-server.test.ts --test-name-pattern='(question|retained|watch|event|idempot|cursor|Codex wake|poll)').
- Required package check: passed; tsc passed, with 3 existing optional-chain warnings in question-state.ts.
- Fast state-writer gate: passed (bun scripts/verify-gjc-state-writers.ts --fail).
- Remote exact-head affected-path/product checks: green.
- PR contract bootstrap and Validate exact-head PR contract: failed truthfully because the body is needs-human and no authenticated approving GitHub review exists for this exact head.
No code was pushed, no head was changed, and no merge/release was performed. Resolve the blockers and obtain an independent authenticated exact-head approval before changing the verdict to merge-approved.
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Fix-forward re-review for exact head 1108151 over base 7e256d6.
The implementation blockers from the prior review are addressed by this head: durable question.answered outbox emission, scheduler/event continuation cursors, retained empty-scan advancement, watcher abort/teardown handling, same-key terminal-uncertain answer recovery, Codex wake shutdown ownership, stable lifecycle event IDs, deletion recovery snapshots, and compactTransaction wiring.
Local evidence: focused coordinator/question/watch/idempotency/cursor/Codex-wake tests passed (18 targeted coordinator tests; 140/151 in the broader coordinator batch, with 8 real-broker failures caused by the missing local native addon); coding-agent check passed with three existing warnings; state-writer gate passed.
The remaining contract gate is owner confirmation: no independent authenticated approval for this exact head is present, so merge remains blocked.
gajae.pr-review-verdict.v1 needs-human sha256:18ed4b5d1adc4bed4b038fd7588c7ccb6ec3cd22179fec9fc2d59eb7fcf007ac reviewer:critic reviewer-id:Yeachan-Heo evidence:fix-forward-focused-validation
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Fix-forward re-review for exact head 1108151 over base 7e256d6.
The implementation blockers from the prior review are addressed by this head: durable question.answered outbox emission, scheduler/event continuation cursors, retained empty-scan advancement, watcher abort/teardown handling, same-key terminal-uncertain answer recovery, Codex wake shutdown ownership, stable lifecycle event IDs, deletion recovery snapshots, and compactTransaction wiring.
Local evidence: focused coordinator/question/watch/idempotency/cursor/Codex-wake tests passed (18 targeted coordinator tests; 140/151 in the broader coordinator batch, with 8 real-broker failures caused by the missing local native addon); coding-agent check passed with three existing warnings; state-writer gate passed.
The remaining contract gate is owner confirmation: no independent authenticated approval for this exact head is present, so merge remains blocked.
gajae.pr-review-verdict.v1 needs-human sha256:221388475737b1b0379f199dbbde625e8fee4ff094b8ff15f9e3c878e5c76199 reviewer:critic reviewer-id:Yeachan-Heo evidence:fix-forward-focused-validation
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head independent code review for 1108151 over base 7e256d6.
The prior implementation blockers are closed by this head: durable question.answered outbox publication, fair scheduler and event-session continuation cursors, retained empty-scan cursor persistence, watch abort/teardown handling, same-key terminal-uncertain answer recovery, Codex wake shutdown ownership, stable lifecycle event identifiers, deletion recovery snapshots, and compactTransaction wiring.
Local evidence: 18 targeted coordinator question/retained/watch/event/idempotency/cursor/Codex-wake tests passed; coding-agent package check passed with three existing optional-chain warnings; state-writer gate passed. The broader coordinator batch had 140/151 passing, with the remaining real-broker failures caused by the missing local native addon rather than this change. Remote coordinator MCP shard passed on exact head; affected-path package checks are still running.
No independent authenticated approving GitHub review exists for this exact head. I therefore cannot truthfully self-approve or issue merge-approved. The PR remains blocked on owner confirmation, with the exact body verdict digest bound to the current diff.
gajae.pr-review-verdict.v1 needs-human sha256:221388475737b1b0379f199dbbde625e8fee4ff094b8ff15f9e3c878e5c76199 reviewer:critic reviewer-id:Yeachan-Heo evidence:exact-head-independent-review-and-focused-validation
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head fix-forward review for b73cbcc over base 7e256d6.
The red-team follow-up found and this head closes the final two P1s: wrapped retained-delivery discovery now returns earlier-session claims after a later-session lexical cursor, and a broker_closed deletion retry re-runs endpoint terminal/deletion verification before local cleanup. Prior coordinator durability, cursor, watcher, idempotency, wake-lifecycle, stable-ID, deletion-snapshot, and compaction findings remain addressed.
Local exact-head evidence: targeted coordinator question/retained/watch/event/idempotency/cursor/Codex-wake tests pass; coding-agent package check passes with three existing optional-chain warnings; state-writer gate passes. The broader coordinator batch is environment-limited only by the unavailable local native addon in real-broker tests. Remote coordinator MCP validation passed on the prior exact head; fresh CI for this final head is running.
No independent authenticated approving GitHub review exists for this exact head. I cannot truthfully self-approve or issue merge-approved. Merge remains blocked pending authenticated owner approval and green required CI.
gajae.pr-review-verdict.v1 needs-human sha256:abf7b3384450ff58c2f997f6f881a546c88a288064fa74089c26589bf8e405fd reviewer:critic reviewer-id:Yeachan-Heo evidence:final-fix-forward-validation
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head terminal review for b73cbcc over immutable base 7e256d6.
Technical review status: CLEAR after the wrapped-retained-delivery and broker-closed deletion-revalidation fixes. Focused coordinator MCP question/retained/watch/event/idempotency/cursor/Codex-wake tests pass; package check and state-writer gate pass. Latest product and virtual-integration CI are green. The only remaining blocker is the repository contract: no independent authenticated approval exists for this exact head, so merge-approved would be false.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED.
gajae.pr-review-verdict.v1 needs-human sha256:abf7b3384450ff58c2f997f6f881a546c88a288064fa74089c26589bf8e405fd reviewer:critic reviewer-id:Yeachan-Heo evidence:final-exact-head-review
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
b73cbcc to
7cfd5d1
Compare
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head rebase review for 7cfd5d1 over current dev base 06f0d4d.
The authorized branch was rebased onto current origin/dev. Wrapped retained-delivery recovery and broker-closed deletion revalidation are preserved. Focused Coordinator MCP question/retained/watch/event/idempotency/cursor/Codex-wake tests pass; coding-agent check and state-writer gate pass; local virtual integration validation passed against the reachable green authority base. The PR remains technically clear.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: the only remaining merge blocker is an independent authenticated APPROVED review from the requested reviewer probepark targeting this exact head. I will not fabricate or self-approve that review.
gajae.pr-review-verdict.v1 needs-human sha256:abf7b3384450ff58c2f997f6f881a546c88a288064fa74089c26589bf8e405fd reviewer:critic reviewer-id:Yeachan-Heo evidence:rebased-current-dev-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
|
Rebased exact head 7cfd5d1 is ready for the requested probepark review. Current dev base: 06f0d4d All affected-path checks, coordinator MCP tests, coding-agent check, native build, state gates, and virtual integration are green. The exact-head contract check is intentionally blocked only by — |
7cfd5d1 to
18a6ebc
Compare
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head latest-dev rebase review for 18a6ebc over current dev 11a156b.
Coordinator event delivery, wrapped retained discovery, deletion recovery, watcher lifecycle, answer idempotency, Codex wake ownership, stable lifecycle IDs, and compaction fixes are preserved through the rebase. Current focused tests, coding-agent check, state-writer gate, native validation, and virtual integration are green. No technical blocker remains.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark is the sole requested reviewer and must approve this exact head. No other reviewer was added; self-approval is not valid.
gajae.pr-review-verdict.v1 needs-human sha256:abf7b3384450ff58c2f997f6f881a546c88a288064fa74089c26589bf8e405fd reviewer:critic reviewer-id:probepark evidence:latest-dev-exact-head-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head contract correction for 18a6ebc over 11a156b.
Recomputed current binary diff digest: abf7b3384450ff58c2f997f6f881a546c88a288064fa74089c26589bf8e405fd. Current focused coordinator tests, coding-agent check, state-writer gate, and the completed product/state validation evidence are bound to this head. Native validation and remaining affected-path jobs are still running.
The single PR verdict is now bound to the requested reviewer identity. No approval is being fabricated; probepark must submit an authenticated APPROVED review for this exact head before merge.
gajae.pr-review-verdict.v1 needs-human sha256:abf7b3384450ff58c2f997f6f881a546c88a288064fa74089c26589bf8e405fd reviewer:human reviewer-id:probepark evidence:latest-dev-focused-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
18a6ebc to
9d9d3f9
Compare
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head current-dev rebase review for 9d9d3f9 over dev 7a920df.
The Coordinator fixes are preserved through the latest rebase. Current focused tests pass after a one-test transient retry; coding-agent check and state-writer gate pass. The exact binary diff digest was recomputed against the live dev base. Fresh CI is being driven for this head.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must provide an authenticated APPROVED review targeting this exact head. No other reviewer was added and no approval is fabricated.
gajae.pr-review-verdict.v1 needs-human sha256:abf7b3384450ff58c2f997f6f881a546c88a288064fa74089c26589bf8e405fd reviewer:human reviewer-id:probepark evidence:current-dev-exact-head-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
9d9d3f9 to
63ed4e1
Compare
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head corruption-fallback fix review for 63ed4e1 over current dev b8880e8.
The concrete Architect caveat is repaired: corrupt journal input during an aborted watch now returns event_snapshot_unavailable instead of masquerading as an empty timed-out snapshot, with a discriminating regression test. Focused Coordinator tests now pass (19 targeted tests), coding-agent check and state-writer gate pass. This head supersedes all prior review/CI evidence.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must authenticate an APPROVED review for this exact head. No self-approval or stale approval is being counted.
gajae.pr-review-verdict.v1 needs-human sha256:2aa827d7f02bb43af3cd5f3f98118e1f8dce8a3ec48da9b53588ad85a7905bcb reviewer:human reviewer-id:probepark evidence:corruption-fallback-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head adversarial repair review for 85fe104 over current dev b8880e8.
Repaired concrete P1 findings from the adversarial review:
- timeout fallback now rejects duplicate event IDs and stale watermarks instead of fabricating an empty success;
- completed prompt requests recover from canonical state without allocating a second turn, while uncertain prompt retries remain redispatchable;
- broker-close
not_foundon an existing deletion intent advances recovery tobroker_closedso cleanup cannot wedge after a close-before-phase crash.
Focused Coordinator tests pass (19 targeted tests), package check passes, and state-writer gate passes. This head supersedes all prior evidence.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must provide an authenticated APPROVED review targeting this exact head. No self-approval or stale review is counted.
gajae.pr-review-verdict.v1 needs-human sha256:6d876b888195e9142eba6c23a75766642499e5e3f77e42dd2f60ffa5eccbfa10 reviewer:human reviewer-id:probepark evidence:adversarial-recovery-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head final adversarial repair review for a3f7009 over current dev b8880e8.
Closed the remaining same-key crash-replay P1: claimCanonicalPrompt now recognizes a completed canonical request, returns its committed runtime receipt, and all prompt call sites skip remote redispatch while recordAcceptedPrompt reuses the canonical turn. The timeout fallback duplicate-ID/stale-watermark and broker-close deletion-recovery repairs remain in this head. Focused Coordinator tests pass (19 targeted tests), coding-agent check passes, and state-writer gate passes.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must provide an authenticated APPROVED review for this exact head. No self-approval or stale review is counted.
gajae.pr-review-verdict.v1 needs-human sha256:3f486e429c62a49bd4e347be20d166f432edb212c252cac9bae1576017be0de5 reviewer:human reviewer-id:probepark evidence:final-adversarial-recovery-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head event-hardening review for ef55be2 over current dev b8880e8.
Closed the latest adversarial findings: stale timeout snapshots now return cursor_ahead with the preserved watermark; runtime acknowledgements update canonical delivery state and do not republish once acknowledged; journal parsing rejects unknown event kinds; outbox normalization rejects duplicate public_event_id values; and registration/start/activation event IDs hash caller idempotency keys before persistence/public delivery. Earlier corrupt fallback, deletion retry, and completed prompt replay repairs remain in this head.
Focused Coordinator tests pass (19 targeted tests), package check passes, and state-writer gate passes. Fresh CI is required for this exact head.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must authenticate an APPROVED review for this exact head. No self-approval or stale review is counted.
gajae.pr-review-verdict.v1 needs-human sha256:78760dfe03a25f8660157e8f29043f16680d34f4147adf2ffb4d7f5d4fd95979 reviewer:human reviewer-id:probepark evidence:event-identity-ack-recovery-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head atomic-acknowledgement review for 2a5d7a7 over current dev b8880e8.
Closed the remaining durability findings: runtime acknowledgement canonical delivery and turn.acknowledged outbox intent are now committed in one transaction before legacy projections; repeated reconciliation reuses the deterministic outbox intent and retries export without duplicate events; namespace-wide duplicate public_event_id values are rejected; and unknown outbox kinds fail closed before journal export. All earlier cursor, journal corruption, prompt replay, deletion recovery, and ID-redaction repairs remain present.
Focused Coordinator tests pass (19 targeted tests), package check passes, and state-writer gate passes. Fresh exact-head CI and adversarial review are required for this head.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must authenticate an APPROVED review for this exact head. No self-approval or stale review is counted.
gajae.pr-review-verdict.v1 needs-human sha256:9c5210e9a6d1d3294bb479e40486c2c894af665e035497503d80445a12e15836 reviewer:human reviewer-id:probepark evidence:atomic-ack-cross-session-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head projection-recovery review for 1407fc9 over current dev b8880e8.
Closed the remaining replay durability findings: completed prompt replay repairs missing legacy projections and drains retained delivery before returning; acknowledgement publication fails closed if the canonical turn is absent; completed answer replay opportunistically exports pending question.answered intent. The atomic acknowledgement/outbox, namespace-wide duplicate-ID, unknown-kind, cursor, journal, deletion, prompt replay, and idempotency-key redaction fixes remain in this head.
Focused Coordinator tests pass (19 targeted tests), package check passes, and state-writer gate passes. Fresh exact-head adversarial review and CI are required for this head.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must authenticate an APPROVED review for this exact head. No self-approval or stale review is counted.
gajae.pr-review-verdict.v1 needs-human sha256:3508c0b8433593c9f8f0acc00fc682758348cf7092f94114e31e35b8d0bfebe5 reviewer:human reviewer-id:probepark evidence:projection-recovery-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head isolation/replay repair review for 529604b over current dev b8880e8.
Closed the latest P1s: completed prompt replay now repairs all legacy projections and drains retained delivery even when a turn projection exists; nonterminal projection repair preserves sidecar source/live/activity; retained export rejects payload session ownership mismatches; and conflicting stable-ID reuse fails closed instead of silently dropping a lifecycle event. Atomic acknowledgement/outbox, cross-session duplicate detection, unknown-kind validation, cursor/journal, deletion, prompt replay, and secret-redaction repairs remain present.
Focused Coordinator tests pass (19 targeted tests), package check passes, and state-writer gate passes. Fresh exact-head adversarial review and CI are required for this head.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must authenticate an APPROVED review for this exact head. No self-approval or stale review is counted.
gajae.pr-review-verdict.v1 needs-human sha256:4c6e35c97528f0f275b148bfbab63b333f5a037c58409bc7b7dda6dad2013970 reviewer:human reviewer-id:probepark evidence:isolation-replay-final-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head prompt/report/scheduler recovery review for 909e183 over current dev b8880e8.
Closed the latest replay/isolation findings: completed prompt claims are resolved before active-turn rejection; report status retries recover canonical operation_id/report IDs; WAL session discovery scans session transaction directories beyond the roster; fresh broker-close not_found advances its newly persisted deletion intent; and terminal transitions emit a durable session.state_changed intent. Existing projection sidecar preservation, payload ownership, stable-ID conflict, atomic ack/outbox, cross-session duplicate, unknown-kind, cursor/journal, deletion, prompt, report, and secret-redaction repairs remain present.
Focused Coordinator tests pass (19 targeted tests), package check passes, and state-writer gate passes. Fresh exact-head adversarial review and CI are required for this head.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must authenticate an APPROVED review for this exact head. No self-approval or stale review is counted.
gajae.pr-review-verdict.v1 needs-human sha256:e686e7c599cb26ec3c85e7aed8d5375e9cd295f7765ce18a3b863c565000a91b reviewer:human reviewer-id:probepark evidence:prompt-report-scheduler-recovery-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head replay-isolation closure review for aed166a over current dev b8880e8.
Closed the latest architectural blockers: remote-started prompt claims are considered before active-turn rejection; report status uses recoverable in-progress idempotency and operation-id lookup including turn-only calls; scheduler discovery includes WAL transaction directories; persisted deletion phases remain monotonic and operation receipts seal completed cleanup; retained entity IDs are checked against namespace-wide canonical ownership; and stable-ID replay compares event identity fields. Terminal session lifecycle intents and projection sidecar preservation remain durable.
Focused Coordinator tests pass (19 targeted tests), package check passes, and state-writer gate passes. Fresh exact-head adversarial review and CI are required for this head.
REQUEST_CHANGES / OWNER_CONFIRMATION_REQUIRED: probepark remains the sole requested reviewer and must authenticate an APPROVED review for this exact head. No self-approval or stale review is counted.
gajae.pr-review-verdict.v1 needs-human sha256:1555161e433cabf6c4e420022d324d7d5e900a53f859cf003977d99793ac721f reviewer:human reviewer-id:probepark evidence:replay-isolation-closure-validation
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
|
Current exact-head latest-dev rebase for 6be3768 over 06b8761. The sessionless-report journal event and I/O error classification repairs were rebased onto the latest dev tip without conflicts. Complete Coordinator suite passes 131/131 (521 expectations); coding-agent check and state-writer gate pass. The exact diff digest remains The old exact-head CHANGES_REQUESTED review targeted 0bcac13 and is stale. probepark is requested for this exact rebased head. gajae.pr-review-verdict.v1 needs-human sha256:1e450202240cd544ea400fe5687a1e264cd779bbf691f2a80bea897c2b3c8ed7 reviewer:human reviewer-id:probepark evidence:latest-dev-sessionless-report-event-io-fix — |
|
Exact-head contract refresh for PR #4702:
Delta audit against the previously approved head
The prior gajae.pr-review-verdict.v1 needs-human sha256:1e450202240cd544ea400fe5687a1e264cd779bbf691f2a80bea897c2b3c8ed7 reviewer:human reviewer-id:probepark evidence:exact-head-6be3768a-rebase-normalized-delta-no-pr-functional-change — |
|
Patch-identity proof for the exact-head refresh:
The current verdict remains needs-human with the exact digest and — |
probepark
left a comment
There was a problem hiding this comment.
Re-affirmed at exact head 6be3768a — approved, carried forward unchanged.
The head moved but the reviewable content did not. The contract digest here is identical to the one my approval was issued against:
sha256:1e450202240cd544ea400fe5687a1e264cd779bbf691f2a80bea897c2b3c8ed7
So this is a re-affirmation rather than a review; I have not re-derived anything.
Both majors remain fixed as verified at 6fffff36: readers split I/O from parse failure (ENOENT to missing, instanceof SyntaxError to corrupt, everything else rethrown with its errno), and sessionless report_status appends a namespace-level report.written keyed by stableId: report-written:${reportId} so a durable blocked report reaches watch_events with replay idempotency. The cleanup-pending reap recovery and complete startup webhook replay are likewise unchanged.
The open minor stands: readJsonFile still returns null for both ENOENT and SyntaxError, so callers cannot distinguish absent from corrupt. Not blocking, worth a follow-up.
Re-affirmed by @probepark — method: recomputed the contract digest at this head and compared it to the digest recorded in the prior verdict. Identical, so the approval carries without re-derivation.
gajae.pr-review-verdict.v1 merge-approved sha256:1e450202240cd544ea400fe5687a1e264cd779bbf691f2a80bea897c2b3c8ed7 reviewer:human reviewer-id:probepark evidence:exact-head-6be3768a-content-digest-identical-to-prior-approved-verdict-carried-forward
6be3768 to
30ec9f8
Compare
|
Live-dev rebase fix-forward for PR #4702:
The previous approved head The previous gajae.pr-review-verdict.v1 needs-human sha256:ea89cbfd31bfa67b89cbd8b2f6fc43c2a07f1ed3c1310158ea4144112a9cf479 reviewer:human reviewer-id:probepark evidence:live-dev-rebase-sessionless-report-event-io-fix — |
|
Live-dev exact-head CI result for
— |
probepark
left a comment
There was a problem hiding this comment.
Re-review at exact head 30ec9f82 — approved. The contract digest moved only because the base advanced; this PR's contribution is unchanged from what I approved at 6be3768a.
I verified that rather than assuming it. The full contract diff against 4ad3726c is 41 lines across three files, and it is exactly the three changes I approved:
1. errno is no longer collapsed (question-state.ts:349-354):
if ((error as NodeJS.ErrnoException).code === "ENOENT") return null;
if (error instanceof SyntaxError) throw new Error("state_corrupt");
throw error;Absent, corrupt, and everything else are now three outcomes instead of two. EACCES, EIO and EMFILE propagate as themselves instead of being reported as corrupt state — which was the whole point, since "corrupt" invites a destructive repair of a file that is merely unreadable.
2. sessionless report_status gets a stable identity (server.ts:6058) — stableId: report-written:${reportId} is attached only when sessionId == null, so the sessionless path has a dedupe key while the session-scoped path keeps its existing identity.
3. the test pins the count, not just the presence (coordinator-mcp-server.test.ts:1759-1762) — watch_events filtered to report.written must have length exactly 1. A duplicate-append regression fails; asserting presence alone would not.
No new findings. My earlier note stands as a follow-up and not a blocker: readJsonFile elsewhere still maps both ENOENT and SyntaxError to null, which is the same conflation this PR fixed here. Worth the same treatment in a separate change.
Reviewed by @probepark — method: recomputed the contract diff at the new base and compared it line by line against the change set I approved at 6be3768a rather than treating a digest change as new content; re-checked that the errno split rethrows the residual case and that the test asserts an exact count.
gajae.pr-review-verdict.v1 merge-approved sha256:ea89cbfd31bfa67b89cbd8b2f6fc43c2a07f1ed3c1310158ea4144112a9cf479 reviewer:human reviewer-id:probepark evidence:exact-head-30ec9f82-contract-diff-identical-to-approved-6be3768a-content-base-advanced-only
30ec9f8 to
64e3049
Compare
64e3049 to
f97aeae
Compare
|
Ancestry hard-block fix-forward completed.
The Coordinator durable event reconciliation delta is preserved as the same three-file, seven-line patch. Focused Coordinator suite passes 124/124; coding-agent package check and state-writer gate pass. The prior — |
Superseded by the exact-head Coordinator repair review and subsequent fix-forward rebases; retain only current exact-head reviewer evidence.
f97aeae to
795565f
Compare
|
#4768 is merged; PR #4702 has been rebased onto current
The prior approval targeted — |
|
The #4768 dependency is merged. PR #4702 is rebased onto current dev at exact head Focused Coordinator + MCP baseline suites pass 140/140; package check and state-writer gate pass. Fresh CI is running. — |
|
Post-#4768 exact-head review reminder: current PR #4702 head is The full affected Dev CI run — |
Emit durable journal events for sessionless reports and distinguish missing/corrupt state from operational read failures across Coordinator and Codex readers. Tested: complete Coordinator suite (131 pass) Tested: coding-agent check and state-writer gate
795565f to
1d90fd3
Compare
|
Latest-dev drift fix-forward completed after
The prior exact-head approval is stale after this push. — |
Superseded by a later force-pushed head; exact-head approval must be re-submitted for the current commit.
probepark
left a comment
There was a problem hiding this comment.
Re-posting at exact head 1d90fd39. My prior approval was dismissed, and the content it covered is unchanged, so this reaffirms it rather than re-deriving findings.
I verified that rather than assuming it. All three hunks I approved are present at this head:
1. errno is not collapsed — question-state.ts:349-354:
if ((error as NodeJS.ErrnoException).code === "ENOENT") return null;
if (error instanceof SyntaxError) throw new Error("state_corrupt");
throw error;Absent, corrupt and everything else stay three outcomes. EACCES, EIO and EMFILE propagate as themselves rather than being reported as corrupt state, which matters because "corrupt" invites a destructive repair of a file that is merely unreadable.
2. sessionless report_status has a stable identity — server.ts:6058, stableId: report-written:${reportId} applied only when sessionId == null, leaving the session-scoped path's identity alone.
3. the test pins the count — coordinator-mcp-server.test.ts:1759-1762 requires watch_events filtered to report.written to have length exactly 1. A duplicate-append regression fails; asserting presence alone would not.
No new findings.
For the record on state: @Yeachan-Heo's changes-requested is bound to ceb5153b (2026-08-19T18:52Z) and the head has since moved to 1d90fd39. If something in that review is still outstanding at this head, say which line and I will look — I would rather re-examine than have a stale block and a fresh approval sit next to each other without explanation.
My earlier follow-up also still stands as a non-blocker: readJsonFile elsewhere maps both ENOENT and SyntaxError to null, which is the same conflation this PR fixes here. Worth the same treatment in a separate change.
Reviewed by @probepark — method: reaffirmation after dismissal. Re-read each of the three approved hunks at the exact current head to confirm the content is unchanged rather than trusting the digest match alone.
gajae.pr-review-verdict.v1 merge-approved sha256:ea89cbfd31bfa67b89cbd8b2f6fc43c2a07f1ed3c1310158ea4144112a9cf479 reviewer:human reviewer-id:probepark evidence:exact-head-1d90fd39-reaffirmation-after-dismissal-all-three-approved-hunks-verified-present-unchanged
|
Terminal disposition for PR #4702:
The Coordinator durable runtime-event repair is merged to — |
The managed assistant snapshot shell rebuilt provider responses from an allowlisted shape but dropped the provider safety-stop kind before AgentSession persistence. That made terminal classification and the configured alternate-model hint unreachable even though provider envelopes and stopReason were correct. Preserve only the exact provider-owned safety-stop literal on errored turns, keep runtime-authored local diagnostics identity-gated, and route the regression through focused PR coverage plus one normal coding-agent shard without duplicate push tasks. Issue: #4777 Confidence: high Scope-risk: medium Reversibility: simple revert Tested: focused provider, managed-boundary, session reopen, routing, fresh-process, package check, and typecheck suites Not-tested: full repository suite and release flow Directive: do not include #4735, #4702, unrelated skill-discovery failures, or release work
The managed assistant snapshot shell rebuilt provider responses from an allowlisted shape but dropped the provider safety-stop kind before AgentSession persistence. That made terminal classification and the configured alternate-model hint unreachable even though provider envelopes and stopReason were correct. Preserve only the exact provider-owned safety-stop literal on errored turns, keep runtime-authored local diagnostics identity-gated, and route the regression through focused PR coverage plus one normal coding-agent shard without duplicate push tasks. Issue: #4777 Confidence: high Scope-risk: medium Reversibility: simple revert Tested: focused provider, managed-boundary, session reopen, routing, fresh-process, package check, and typecheck suites Not-tested: full repository suite and release flow Directive: do not include #4735, #4702, unrelated skill-discovery failures, or release work
What
Coordinator MCP now reconciles runtime terminal receipts and structured workflow blockers into its durable long-poll event journal. The branch also hardens retained-delivery ordering, runtime provenance, public error mapping, queue/report transitions, answer validation, status scope, and controller documentation.
Why
Hermes and other controllers could drive GJC turns but could not reliably observe runtime completion or a newly opened structured blocker through
watch_eventsalone. The previous recovery and public response paths also left correctness and public-contract gaps under audit.Behavior and recovery
gjc_coordinator_watch_eventsprojects terminal, waiting, and question transitions as bounded public journal events.Related context
gjc setup hermesintegration; it is not a separate Hermes-only transport.Testing
bun test packages/coding-agent/test/coordinator-mcp-server.test.ts packages/coding-agent/test/bot-integration-docs.test.ts packages/coding-agent/test/coordinator-mcp-policy.test.tsbun --cwd=packages/coding-agent run checkbun scripts/verify-gjc-state-writers.ts --failEnvironment
Verified on macOS arm64 with Bun workspace tooling. The change is API/state-machine focused; no platform-specific external transport is added.
GJC verdict
devbun checkpasses forpackages/coding-agentDelta audit: 6fffff3..6be3768 only normalizes three auth-account files to the base versions (CHANGELOG and accounts-cli hashes equal base; accounts-cli-errors.test.ts is restored from base). The Coordinator functional diff and digest are unchanged: sha256:1e450202240cd544ea400fe5687a1e264cd779bbf691f2a80bea897c2b3c8ed7.
Human GitHub approval is required again because the branch was rebased onto live dev 012f62b.